Conversation
WalkthroughThe changes introduce and propagate a default Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Component
participant $setup
participant getDesignConfig
participant getCustomProps
participant renderComponent
User->>Component: Uses Alert component
Component->>$setup: Calls setup with props/context
$setup->>getDesignConfig: Fetch design config (global & component)
$setup->>getCustomProps: Retrieve user-passed props
$setup->>renderComponent: Merge design props (e.g., center: true) with user props
renderComponent-->>Component: Rendered with merged props
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
examples/sites/demos/pc/app/config-provider/webdoc/config-provider.jsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-vue". (The package "eslint-plugin-vue" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-vue" was referenced from the config file in ".eslintrc.js » @antfu/eslint-config » @antfu/eslint-config-vue". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. packages/vue-common/src/adapter/vue2.7/index.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-vue". (The package "eslint-plugin-vue" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-vue" was referenced from the config file in ".eslintrc.js » @antfu/eslint-config » @antfu/eslint-config-vue". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. packages/vue-common/src/adapter/vue2/index.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-vue". (The package "eslint-plugin-vue" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-vue" was referenced from the config file in ".eslintrc.js » @antfu/eslint-config » @antfu/eslint-config-vue". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🧰 Additional context used🧬 Code Graph Analysis (2)packages/vue-common/src/adapter/vue2.7/index.ts (3)
packages/vue-common/src/adapter/vue2/index.ts (3)
🪛 Biome (1.9.4)packages/vue-common/src/adapter/vue2.7/index.ts[error] 35-35: Change to an optional chain. Unsafe fix: Change to an optional chain. (lint/complexity/useOptionalChain) packages/vue-common/src/adapter/vue2/index.ts[error] 44-46: Change to an optional chain. Unsafe fix: Change to an optional chain. (lint/complexity/useOptionalChain) ⏰ Context from checks skipped due to timeout of 90000ms (1)
🔇 Additional comments (6)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
WalkthroughThis PR introduces a feature to the design-config that allows global configuration components to accept arbitrary attributes. It includes updates to various Vue adapter files to support this functionality, as well as modifications to demo and test files to verify the new behavior. Changes
|
![petercat-assistant[bot]](https://avatars.githubusercontent.com/in/875736?s=60&v=4){kind=small}
|
|
||
| Object.keys(designProps).forEach((key) => { | ||
| // 用户没有配置的属性才进行覆盖 | ||
| if (!Object.prototype.hasOwnProperty.call(customProps, key)) { |
There was a problem hiding this comment.
Ensure that Object.prototype.hasOwnProperty.call is used correctly to avoid potential issues with objects that may not inherit from Object.prototype. This is a critical check to prevent unexpected behavior.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
packages/vue-common/src/index.ts (3)
159-170:getDesignConfig– handle case-mismatch & null component names
getComponentName()may return an empty string for anonymous components, leading to an undefined lookup inglobalDesignConfig.components.
Also, component names in user config might be written in different cases (e.g.alert,Alert,ALERT).Consider normalising both sides to the same case and early-returning when no name is found:
- const designConfig = globalDesignConfig?.components?.[getComponentName().replace($prefix, '')] + const compName = getComponentName()?.replace($prefix, '') || '' + const designConfig = + compName && + Object.keys(globalDesignConfig?.components ?? {}).find( + (key) => key.toLowerCase() === compName.toLowerCase() + ) + ? globalDesignConfig.components[ + Object.keys(globalDesignConfig.components).find( + (key) => key.toLowerCase() === compName.toLowerCase() + ) + ] + : undefinedThis prevents subtle “config not applied” bugs when users write
alertinstead ofAlert.
172-201:$setup– avoid recomputation ofcustomPropsinside computed
getCustomProps()relies ongetCurrentInstance(); when called during setup it is cheap, but invoking it on every component instantiation is unnecessary.
Move the call outside thedesignPropsloop to compute once:- const designProps = designConfig?.props - if (designProps) { - // 获取用户传递的props - const customProps = getCustomProps() + const designProps = designConfig?.props + const customProps = getCustomProps() + if (designProps) {Negligible performance gain but improves readability.
202-208:mergeClasshelper leaks tailwind-merge to every consumer
mergeClasscallscustomDesignConfig.twMergedirectly. If a user replacestwMergelater (lazy injection), already-imported modules will still hold the old reference.Export it as a function capturing the current
twMergeat call time:-export const mergeClass = (...cssClasses) => customDesignConfig.twMerge(stringifyCssClass(cssClasses)) +export const mergeClass = (...cssClasses) => + (customDesignConfig.twMerge ?? ((v) => v))(stringifyCssClass(cssClasses))Prevents runtime crashes when
twMergeis swapped or not provided.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
examples/sites/demos/pc/app/config-provider/base-composition-api.vue(1 hunks)examples/sites/demos/pc/app/config-provider/base.vue(1 hunks)examples/sites/demos/pc/app/config-provider/basic.spec.ts(1 hunks)packages/vue-common/src/adapter/vue2.7/index.ts(1 hunks)packages/vue-common/src/adapter/vue2/index.ts(1 hunks)packages/vue-common/src/adapter/vue3/index.ts(2 hunks)packages/vue-common/src/index.ts(2 hunks)packages/vue/package.json(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
packages/vue-common/src/adapter/vue2/index.ts (2)
packages/vue-common/src/adapter/vue2.7/index.ts (1)
getCustomProps(41-44)packages/vue-common/src/adapter/vue3/index.ts (1)
getCustomProps(47-50)
packages/vue-common/src/adapter/vue2.7/index.ts (2)
packages/vue-common/src/adapter/vue2/index.ts (1)
getCustomProps(52-55)packages/vue-common/src/adapter/vue3/index.ts (1)
getCustomProps(47-50)
packages/vue-common/src/adapter/vue3/index.ts (2)
packages/vue-common/src/adapter/vue2/index.ts (1)
getCustomProps(52-55)packages/vue-common/src/adapter/vue2.7/index.ts (1)
getCustomProps(41-44)
packages/vue-common/src/index.ts (1)
packages/vue-common/src/adapter/vue3/index.ts (3)
getComponentName(52-61)getCustomProps(47-50)renderComponent(25-39)
🪛 Biome (1.9.4)
packages/vue-common/src/adapter/vue3/index.ts
[error] 35-35: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: PR E2E Test (pnpm test:e2e3)
🔇 Additional comments (6)
packages/vue-common/src/adapter/vue2/index.ts (1)
52-55: Well-implemented utility function to support arbitrary attributesThis
getCustomPropsfunction elegantly retrieves the component instance's props data, allowing access to user-defined props. This is essential for the design-config feature to support arbitrary attributes in global configuration components.The implementation correctly handles potential undefined values using optional chaining and provides a fallback empty object. It's also consistent with the similar implementations in Vue 2.7 and Vue 3 adapters.
packages/vue/package.json (1)
240-240: LGTM - Minor whitespace changeThe only change is the removal of a trailing newline, which is a trivial formatting change with no functional impact.
examples/sites/demos/pc/app/config-provider/base-composition-api.vue (1)
56-58: Well-implemented design configuration attributeAdding the
center: trueprop to the Alert component's design configuration demonstrates the new arbitrary attribute feature. This change makes the Alert component's text centered by default when using this design configuration.The implementation is clean and properly structured within the existing configuration object.
examples/sites/demos/pc/app/config-provider/basic.spec.ts (1)
13-15: Good test coverage for the new featureThis test properly verifies that the Alert component's text is centered as a result of the
center: truedesign configuration property. The test confirms that the CSStext-alignproperty is correctly set tocenter.The comment "验证文字居中" (verifying text centering) clearly documents the purpose of this test case.
examples/sites/demos/pc/app/config-provider/base.vue (1)
63-66: Nice addition – validates the new design-config capabilityAdding
center: trueunderAlert.propsshowcases the arbitrary-attribute feature nicely and should read well in demos.Once the Vue 2 adapters forward
customDesignProps(see previous comment) this demo will work across all targets.packages/vue-common/src/adapter/vue3/index.ts (1)
25-39: Ordering & duplication guard in the merged‐prop spread
...attrsis placed before...customDesignProps.
Because Vue normalises incoming attrs into props when names match, a user could still override a design default via an attribute instead of a prop, so this order is correct – keep it. 👍However,
propsandcustomDesignPropsare independent spreads and may contain the same key; the later spread (customDesignProps) will win.
That contradicts the rule implemented in$setup(“only override when the user did not set the prop”).
Consider merging before callinghto ensure the carefully-filtered object is passed only once:- { ref: 'modeTemplate', ...props, ...attrs, ...customDesignProps, ...extend }, + { + ref: 'modeTemplate', + ...attrs, + ...customDesignProps, + ...props, // user props last -> guaranteed priority + ...extend + },Double-check with a simple unit test that a runtime prop (
<tiny-alert center="false" />) still overrides the design default.🧰 Tools
🪛 Biome (1.9.4)
[error] 35-35: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
| export const getCustomProps = () => { | ||
| const instance = hooks.getCurrentInstance()?.proxy | ||
| return instance?.$options?.propsData || {} | ||
| } |
There was a problem hiding this comment.
getCustomProps added, but the surrounding adapter is still oblivious to the new customDesignProps flow
getCustomProps itself looks fine, yet the Vue 2.7 adapter never forwards the merged customDesignProps object that $setup now passes in.
As a result, any default props coming from the global design-config are silently ignored in a Vue 2.7 runtime, breaking the very feature this PR introduces (e.g. the new Alert.center default will not work).
Minimal patch (works for both Vue 2 and Vue 2.7 adapters):
export const renderComponent = ({
view = null as any,
component = null as any,
props,
+ customDesignProps = {},
context: { attrs, listeners: on, slots },
extend = {}
}) => {
return () =>
hooks.h(
(view && view.value) || component,
- Object.assign({ props, attrs, [extend.isSvg ? 'nativeOn' : 'on']: on, scopedSlots: { ...slots } }, extend)
+ Object.assign(
+ { props: { ...customDesignProps, ...props }, attrs, [extend.isSvg ? 'nativeOn' : 'on']: on, scopedSlots: { ...slots } },
+ extend
+ )
)
}Key points
- Keep the existing ordering so user props still win over design defaults.
- Provide a default
{}to stay backwards-compatible with callers that do not yet supply the parameter.
Please mirror the same change in packages/vue-common/src/adapter/vue2/index.ts to keep all Vue-2 builds consistent.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const getCustomProps = () => { | |
| const instance = hooks.getCurrentInstance()?.proxy | |
| return instance?.$options?.propsData || {} | |
| } | |
| export const renderComponent = ({ | |
| view = null as any, | |
| component = null as any, | |
| props, | |
| customDesignProps = {}, | |
| context: { attrs, listeners: on, slots }, | |
| extend = {} | |
| }) => { | |
| return () => | |
| hooks.h( | |
| (view && view.value) || component, | |
| Object.assign( | |
| { props: { ...customDesignProps, ...props }, attrs, [extend.isSvg ? 'nativeOn' : 'on']: on, scopedSlots: { ...slots } }, | |
| extend | |
| ) | |
| ) | |
| } |
| export const getCustomProps = () => { | ||
| const instance = hooks.getCurrentInstance() | ||
| return instance?.vnode?.props || {} | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Prefer instance.props to capture camel-cased prop keys
instance.vnode.props gives you the raw vnode props which may preserve kebab-cased keys and include attrs/events.
instance.props exposes the normalised reactive props object, mapping kebab-case to camelCase – a closer match to the keys declared in designConfig.props.
-export const getCustomProps = () => {
- const instance = hooks.getCurrentInstance()
- return instance?.vnode?.props || {}
-}
+export const getCustomProps = () => {
+ const instance = hooks.getCurrentInstance()
+ return instance?.props || {}
+}This avoids false negatives when a user passes center="false" (kebab) but the design config checks for center (camel).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const getCustomProps = () => { | |
| const instance = hooks.getCurrentInstance() | |
| return instance?.vnode?.props || {} | |
| } | |
| export const getCustomProps = () => { | |
| const instance = hooks.getCurrentInstance() | |
| return instance?.props || {} | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/vue-common/src/adapter/vue2.7/index.ts (1)
36-39:⚠️ Potential issue
customDesignPropsis passed incorrectly to the componentThe current implementation adds
customDesignPropsas a separate property in the object passed to the Vue component, but it needs to be merged with thepropsobject to actually apply the design configuration properties to the component.Apply this fix to properly merge design props with user props:
Object.assign( - { props, attrs, customDesignProps, [extend.isSvg ? 'nativeOn' : 'on']: on, scopedSlots: { ...slots } }, + { props: { ...customDesignProps, ...props }, attrs, [extend.isSvg ? 'nativeOn' : 'on']: on, scopedSlots: { ...slots } }, extend )This ensures that:
- Design configuration properties are applied to the component
- User props take precedence over design defaults (as they should)
- The implementation is consistent with the expected behavior
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/vue-common/src/adapter/vue2.7/index.ts(1 hunks)packages/vue-common/src/adapter/vue2/index.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/vue-common/src/adapter/vue2/index.ts (3)
packages/vue-common/src/index.ts (1)
hooks(412-412)packages/vue-common/src/adapter/vue2.7/index.ts (2)
rootConfig(43-43)getCustomProps(45-48)packages/vue-common/src/adapter/vue3/index.ts (2)
rootConfig(41-45)getCustomProps(47-50)
🪛 Biome (1.9.4)
packages/vue-common/src/adapter/vue2.7/index.ts
[error] 35-35: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
packages/vue-common/src/adapter/vue2/index.ts
[error] 44-46: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: PR E2E Test (pnpm test:e2e3)
🔇 Additional comments (4)
packages/vue-common/src/adapter/vue2/index.ts (2)
37-37: Added new parametercustomDesignPropsfor design configuration supportThe parameter has been added to support arbitrary attributes from design configuration, which aligns with the PR objectives.
60-63:getCustomPropsimplementation looks goodThe function correctly retrieves the current component instance's props data, which will be used for merging with design configuration.
packages/vue-common/src/adapter/vue2.7/index.ts (2)
29-29: Added new parametercustomDesignPropsfor design configuration supportThe parameter has been added to support arbitrary attributes from design configuration, which aligns with the PR objectives.
45-48:getCustomPropsimplementation looks goodThe function correctly retrieves the current component instance's props data, which will be used for merging with design configuration.
| { | ||
| props, | ||
| attrs, | ||
| customDesignProps, | ||
| [extend.isSvg ? 'nativeOn' : 'on']: on, | ||
| ref: 'modeTemplate', | ||
| scopedSlots: { ...slots } | ||
| }, |
There was a problem hiding this comment.
customDesignProps is passed incorrectly to the component
The current implementation adds customDesignProps as a separate property in the object passed to the Vue component, but it needs to be merged with the props object to actually apply the design configuration properties to the component.
Apply this fix to properly merge design props with user props:
Object.assign(
{
- props,
+ props: { ...customDesignProps, ...props },
attrs,
- customDesignProps,
[extend.isSvg ? 'nativeOn' : 'on']: on,
ref: 'modeTemplate',
scopedSlots: { ...slots }
},
extend
)This ensures that:
- Design configuration properties are applied to the component
- User props take precedence over design defaults (as they should)
- The implementation is consistent with the expected behavior
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| { | |
| props, | |
| attrs, | |
| customDesignProps, | |
| [extend.isSvg ? 'nativeOn' : 'on']: on, | |
| ref: 'modeTemplate', | |
| scopedSlots: { ...slots } | |
| }, | |
| { | |
| props: { ...customDesignProps, ...props }, | |
| attrs, | |
| [extend.isSvg ? 'nativeOn' : 'on']: on, | |
| ref: 'modeTemplate', | |
| scopedSlots: { ...slots } | |
| }, |
2 participants
feat(design-config): design-config添加全局配置组件任意属性特性
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Chores